CSHARP-5712: withTransaction API retries too frequently by sanych-sun · Pull Request #1841 · mongodb/mongo-csharp-driver

sanych-sun · 2025-12-19T01:01:30Z

No description provided.

Copilot

Pull request overview

This PR implements the RetryabilityHelper.GetRetryDelayMs method to control retry delays with exponential backoff, addressing the issue of transactions retrying too frequently. The implementation adds jitter to prevent thundering herd problems and includes comprehensive parameter validation.

Key Changes

Adds GetRetryDelayMs method with exponential backoff calculation and random jitter
Removes unused MongoDB.Driver.Core.Connections import from test file
Adds comprehensive test coverage for valid inputs, edge cases, and parameter validation

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/MongoDB.Driver/Core/Operations/RetryabilityHelper.cs	Implements the core retry delay calculation with exponential backoff and jitter
tests/MongoDB.Driver.Tests/Core/Operations/RetryabilityHelperTests.cs	Adds unit tests for the new method and removes unused import

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T01:02:56Z

+            Ensure.IsGreaterThan(backoffMax, backoffInitial, nameof(backoffMax));
+
+            var j = __backoffRandom.NextDouble();
+            return (int)(j * Math.Min(backoffMax, backoffInitial * Math.Pow(backoffBase, attempt - 1)));


The calculation backoffInitial * Math.Pow(backoffBase, attempt - 1) may overflow for large attempt values, which would then be capped by Math.Min. Consider adding overflow protection or documenting the maximum safe attempt value to prevent unexpected behavior.

Suggested change

return (int)(j * Math.Min(backoffMax, backoffInitial * Math.Pow(backoffBase, attempt - 1)));

// compute the largest exponent such that backoffInitial * backoffBase^exponent <= backoffMax

var maxExponent = Math.Log(backoffMax / (double)backoffInitial, backoffBase);

var effectiveExponent = attempt - 1;

double delayWithoutJitter;

if (effectiveExponent >= maxExponent)

{

delayWithoutJitter = backoffMax;

}

else

{

delayWithoutJitter = backoffInitial * Math.Pow(backoffBase, effectiveExponent);

}

return (int)(j * delayWithoutJitter);

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

papafe

Looks good!

papafe · 2026-01-05T18:11:46Z

            }

            var subject = CreateSubject(coreSession: mockCoreSession.Object, clock: mockClock.Object);
+            SetupTransactionState(subject, true);


Is this related to the ticket?

Yes, changes were rolled back. Thank you for noticing this.

BorisDog

Partial review

papafe · 2026-01-06T09:03:42Z

+            var backoffTime = await ExecuteWithTransactionAsync(1);
+
+            var difference = backoffTime - noBackoffTime - TimeSpan.FromSeconds(2.2);
+            difference.Should().BeLessThan(TimeSpan.FromMilliseconds(1));


Are we using a 1 millisecond window here instead of a 1 second because we're "faking" the prose test?

Yep. We do not interact with the server, so timing should be more strict. 1ms could be small though, we might will need to adjust it in future.

BorisDog · 2026-01-07T00:04:12Z

+            var noBackoffTime = await ExecuteWithTransactionAsync(0);
+            var backoffTime = await ExecuteWithTransactionAsync(1);
+
+            var difference = backoffTime - noBackoffTime - TimeSpan.FromSeconds(2.2);


Will
backoffTimeMS.Should().BeApproximately(noBackoffTimeMS + 2200, 1);
be more readable?

BorisDog

Minor suggestion

BorisDog · 2026-01-08T23:17:53Z

                {
-                    return callbackOutcome.Result; // assume callback intentionally ended the transaction
-                }
+                    if (IsTransactionInStartingOrInProgressState(clientSession))


Similar to ShouldRetryTransaction consider ShouldAbortTransaction(out AbortTransactionOptions)

BorisDog

LGTM + minor comment

BorisDog · 2026-01-16T21:19:11Z

                {
-                    return callbackOutcome.Result; // assume callback intentionally ended the transaction
-                }
+                    if(ShouldAbortTransaction(operationContext, clientSession, out var abortOptions))


minor: space after if in both places.

Copilot AI review requested due to automatic review settings December 19, 2025 01:01

sanych-sun requested a review from a team as a code owner December 19, 2025 01:01

sanych-sun requested review from adelinowona and papafe and removed request for adelinowona December 19, 2025 01:01

sanych-sun added the chore Non–user-facing code changes (tests, build scripts, etc.). label Dec 19, 2025

Copilot AI reviewed Dec 19, 2025

View reviewed changes

sanych-sun requested a review from Copilot December 19, 2025 01:11

Copilot AI reviewed Dec 19, 2025

View reviewed changes

Comment thread src/MongoDB.Driver/Core/Operations/RetryabilityHelper.cs Outdated

sanych-sun added feature Adds new user-facing functionality. and removed chore Non–user-facing code changes (tests, build scripts, etc.). labels Dec 19, 2025

sanych-sun requested review from BorisDog and Copilot December 19, 2025 23:21

Copilot AI reviewed Dec 19, 2025

View reviewed changes

Comment thread src/MongoDB.Driver/TransactionExecutor.cs Outdated

papafe reviewed Jan 5, 2026

View reviewed changes

BorisDog requested changes Jan 5, 2026

View reviewed changes

Comment thread src/MongoDB.Driver/Core/Misc/IRandomNumberGenerator.cs Outdated

Comment thread src/MongoDB.Driver/Core/Misc/IRandomNumberGenerator.cs Outdated

Comment thread src/MongoDB.Driver/TransactionExecutor.cs Outdated

Comment thread src/MongoDB.Driver/TransactionExecutor.cs

sanych-sun requested a review from BorisDog January 5, 2026 23:32

BorisDog reviewed Jan 6, 2026

View reviewed changes

Comment thread src/MongoDB.Driver/Core/Misc/IRandomNumberGenerator.cs Outdated

Comment thread src/MongoDB.Driver/Core/Misc/ThreadStaticRandom.cs Outdated

Comment thread src/MongoDB.Driver/TransactionExecutor.cs

Comment thread src/MongoDB.Driver/Core/Misc/RandomNumberGenerator.cs Outdated

sanych-sun requested a review from BorisDog January 6, 2026 04:18

papafe reviewed Jan 6, 2026

View reviewed changes

BorisDog requested changes Jan 7, 2026

View reviewed changes

sanych-sun requested a review from BorisDog January 8, 2026 20:24

BorisDog requested changes Jan 8, 2026

View reviewed changes

sanych-sun requested a review from BorisDog January 15, 2026 19:07

BorisDog approved these changes Jan 16, 2026

View reviewed changes

sanych-sun added 5 commits January 20, 2026 10:03

CSHARP-5712: withTransaction API retries too frequently

f470fbc

pr

dfe18bd

CSHARP-5712: withTransaction API retries too frequently

f013047

Pr

aa77908

pr

ad68d48

sanych-sun added 10 commits January 20, 2026 10:03

pr

e073a58

pr

064f28c

Pr

49647e7

Refactor IRandomStringGenerator and IRandomNumberGenerator into IRandom.

9dae9bd

Pr

0c98f7c

pr

e88ae45

pr

0a18902

pr

cd40b27

pr

549a033

pr

02ea474

sanych-sun force-pushed the csharp5712 branch from 13422a7 to 02ea474 Compare January 20, 2026 18:04

sanych-sun merged commit 22ed045 into mongodb:main Jan 20, 2026
33 of 36 checks passed

sanych-sun deleted the csharp5712 branch January 20, 2026 20:04

-            return (int)(j * Math.Min(backoffMax, backoffInitial * Math.Pow(backoffBase, attempt - 1)));
+            // compute the largest exponent such that backoffInitial * backoffBase^exponent <= backoffMax
+            var maxExponent = Math.Log(backoffMax / (double)backoffInitial, backoffBase);
+            var effectiveExponent = attempt - 1;
+            double delayWithoutJitter;
+            if (effectiveExponent >= maxExponent)
+            {
+                delayWithoutJitter = backoffMax;
+            }
+            else
+            {
+                delayWithoutJitter = backoffInitial * Math.Pow(backoffBase, effectiveExponent);
+            }
+            return (int)(j * delayWithoutJitter);

Conversation

sanych-sun commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

papafe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sanych-sun commented Dec 19, 2025 •

edited

Loading